Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(db): Do not allow creating objects with wrong parent #543

Merged
merged 1 commit into from
Sep 23, 2024

Conversation

nuclearcat
Copy link
Member

If parent is set to same id as object it might cause loops in some of pipeline programs. Block such updates.

If parent is set to same id as object it might cause loops
in some of pipeline programs. Block such updates.

Signed-off-by: Denys Fedoryshchenko <[email protected]>
Copy link
Collaborator

@JenySadadia JenySadadia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am wondering if this check should be in the endpoint handler instead, in api.main just like other checks we have e.g. _verify_user_group_existence.
As this check doesn't concern DB abstraction layer.
What do you think? @nuclearcat

@nuclearcat
Copy link
Member Author

Yes, it make sense, at least i will try to rewrite and add it to endpoint.

@nuclearcat
Copy link
Member Author

@JenySadadia after returning to this question i remember why i moved to db layer. When we submit multiple nodes over PUT /nodes call, we are not unrolling nodes in api.main:

obj_list = await db.create_hierarchy(nodes, Node)
so i dont have access to all individual nodes to verify them easily.
But rather forward it to db layer, this is why i moved parent verification in db layer, as otherwise i will have to duplicate same code and unroll hierarchy twice - in main code too.

@JenySadadia
Copy link
Collaborator

@JenySadadia after returning to this question i remember why i moved to db layer. When we submit multiple nodes over PUT /nodes call, we are not unrolling nodes in api.main:

obj_list = await db.create_hierarchy(nodes, Node)

so i dont have access to all individual nodes to verify them easily.
But rather forward it to db layer, this is why i moved parent verification in db layer, as otherwise i will have to duplicate same code and unroll hierarchy twice - in main code too.

Okay. Usually we create sub-nodes rather than updating them with PUT /nodes request. But, yes, it's good to have validation there as well.
Let's go with db layer in this case.

@nuclearcat nuclearcat added this pull request to the merge queue Sep 23, 2024
Merged via the queue into kernelci:main with commit 07bb3cb Sep 23, 2024
6 checks passed
@nuclearcat nuclearcat deleted the block-wrong-parent branch September 23, 2024 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants